Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix traceback and incorrect completions using SubprocessCompleter #289

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 20, 2020

It looks like using SuppressCompleter() (#224) raises during argument completion which results in some completions getting unexpectedly suppressed. This fixes the issue by making SuppressCompleter() callable checking if the completions should be suppressed prior to checking what type of completer to use.

Problem

Consider this python script:

Python script with completion
#!/usr/bin/env python3

import argparse
from argcomplete import autocomplete

parser = argparse.ArgumentParser()

arg = parser.add_argument(
    '-p', '--print', '--print-description', default=False, action='store_true',
    help='Print the launch description to the console without launching it.')

arg = parser.add_argument(
    'launch_arguments',
    nargs='*',
    help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)")

autocomplete(parser, exclude=['-h', '--help'])

./suppress.py <tab><tab> displays files in the current directory, but I don't want that.

$ ./suppress.py 
file_i_dont_want_compeleted.txt  --print                          suppress.py                      .swp
-p                               --print-description              .suppress.py.swp

Using export _ARC_DEBUG=true I can see the file I don't want completed comes from launch_arguments

# ...
Active actions (L=1): [IntrospectAction(option_strings=[], dest='launch_arguments', nargs='*', const=None, default=None, type=None, choices=None, help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)", metavar=None)]
Activating completion for IntrospectAction(option_strings=[], dest='launch_arguments', nargs='*', const=None, default=None, type=None, choices=None, help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)", metavar=None) <class 'argparse._StoreAction'>
Completions: ['-h', '--help', '-p', '--print', '--print-description', 'file_i_dont_want_compeleted.txt', '.suppress.py.swp', 'suppress.py', '.swp']

So I added SuppressCompleter to launch_arguments.

Python script with suppressed completion
#!/usr/bin/env python3

import argparse
from argcomplete import autocomplete
from argcomplete.completers import SuppressCompleter

parser = argparse.ArgumentParser()

arg = parser.add_argument(
    '-p', '--print', '--print-description', default=False, action='store_true',
    help='Print the launch description to the console without launching it.')

arg = parser.add_argument(
    'launch_arguments',
    nargs='*',
    help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)")
arg.completer = SuppressCompleter()

autocomplete(parser, exclude=['-h', '--help'])

But now with _ARC_DEBUG I see an exception:

# ...
Active actions (L=1): [IntrospectAction(option_strings=[], dest='launch_arguments', nargs='*', const=None, default=None, type=None, choices=None, help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)", metavar=None)]
Activating completion for IntrospectAction(option_strings=[], dest='launch_arguments', nargs='*', const=None, default=None, type=None, choices=None, help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)", metavar=None) <class 'argparse._StoreAction'>
Completer is not callable, trying the readline completer protocol instead
Traceback (most recent call last):
  File "./suppress.py", line 47, in <module>
    autocomplete(parser, exclude=['-h', '--help'])
  File "/home/sloretz/tmp/argcomplete/argcomplete/__init__.py", line 227, in __call__
    completions = self._get_completions(comp_words, cword_prefix, cword_prequote, last_wordbreak_pos)
  File "/home/sloretz/tmp/argcomplete/argcomplete/__init__.py", line 258, in _get_completions
    completions = self.collect_completions(active_parsers, parsed_args, cword_prefix, debug)
  File "/home/sloretz/tmp/argcomplete/argcomplete/__init__.py", line 481, in collect_completions
    completions)
  File "/home/sloretz/tmp/argcomplete/argcomplete/__init__.py", line 447, in _complete_active_option
    next_completion = completer.complete(cword_prefix, i)
AttributeError: 'SuppressCompleter' object has no attribute 'complete'

And the files I don't want completed are still completed, plus the completions for --print are gone.

$ ./suppress.py 
file_i_dont_want_compeleted.txt  suppress.py                      .suppress.py.swp                 .swp

Workaround

I can work around this by making a subclass that is callable.

Python script with callable subclass of SupressCompleter
#!/usr/bin/env python3

import argparse
from argcomplete import autocomplete
from argcomplete.completers import SuppressCompleter


class SuppressCompleterWorkaround(SuppressCompleter):
    """Make SuppressCompleter callable."""

    def __call__(self, *args, **kwargs):
        return tuple()


parser = argparse.ArgumentParser()

arg = parser.add_argument(
    '-p', '--print', '--print-description', default=False, action='store_true',
    help='Print the launch description to the console without launching it.')

arg = parser.add_argument(
    'launch_arguments',
    nargs='*',
    help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)")
arg.completer = SuppressCompleterWorkaround()

autocomplete(parser, exclude=['-h', '--help'])

And that gets the completions I'm looking for:

$ ./suppress.py 
-p                   --print              --print-description

@kislyuk
Copy link
Owner

kislyuk commented Feb 21, 2020

You are correct that there is a bug here. It looks like it's being hit in a branch that handles the active option (the option corresponding to the word under the cursor. However, the preferred solution is to use the same API used elsewhere for SuppressCompleter, and add a type check instead of making it callable:

if isinstance(completer, SuppressCompleter) and completer.suppress():

This should be added to the branch that you are modifying:

if callable(completer):

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz force-pushed the subprocess_completer_callable branch from b19dd66 to adde1df Compare February 24, 2020 18:50
@codecov-io
Copy link

Codecov Report

Merging #289 into master will decrease coverage by 0.59%.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #289     +/-   ##
========================================
- Coverage   82.69%   82.1%   -0.6%     
========================================
  Files           7       7             
  Lines         734     732      -2     
========================================
- Hits          607     601      -6     
- Misses        127     131      +4
Impacted Files Coverage Δ
argcomplete/completers.py 86.95% <50%> (-1.11%) ⬇️
argcomplete/compat.py 93.75% <0%> (-6.25%) ⬇️
argcomplete/__init__.py 90.88% <0%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2c12c3...adde1df. Read the comment docs.

@sloretz
Copy link
Contributor Author

sloretz commented Feb 24, 2020

However, the preferred solution is to use the same API used elsewhere for SuppressCompleter, and add a type check instead of making it callable

Implemented SupressCompleter type check prior to conditional that picks which completer protocol to use in adde1df

@sloretz sloretz changed the title Make SubprocessCompleter callable() Fix traceback and incorrect completions using SubprocessCompleter Feb 24, 2020
@kislyuk kislyuk merged commit 223fa83 into kislyuk:master Mar 10, 2020
@kislyuk
Copy link
Owner

kislyuk commented Mar 10, 2020

Thank you for your contribution! The CI failure was unrelated. I will take care of making a test for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants